fix(chainbase): strengthen signature length validation by Federico2014 · Pull Request #27 · Federico2014/java-tron

Federico2014 · 2026-05-12T12:24:59Z

Summary

Tighten signature length validation across transaction, block and PBFT processing
Gate the new validation on the TVM Osaka proposal parameter to preserve compatibility with historical data
Centralize the bounds in ChainConstant.MIN_SIGNATURE_SIZE (65) and MAX_SIGNATURE_SIZE (68), and the check itself in SignUtils.isValidLength(int size, boolean osakaAllowed); a signature is rejected if size < MIN_SIGNATURE_SIZE or, post-Osaka, size > MAX_SIGNATURE_SIZE
Apply the predicate consistently at every signature-size callsite (TransactionCapsule.checkWeight, BlockCapsule.validateSignature, Wallet.getTransactionApprovedList, RelayService.checkHelloMessage, PbftBaseMessage.analyzeSignature, PbftDataSyncHandler.ValidPbftSignTask)
Harden PBFT quorum accounting so it remains robust under non-canonical signature encodings

Design notes

Size bound: 65..68 instead of strict 65

The post-Osaka window accepts 65..68 bytes rather than enforcing a strict == 65. This is a deliberate compatibility compromise:

A canonical ECDSA signature is exactly 65 bytes (r || s || v).
Historical on-chain data contains signatures with a small amount of trailing padding that ECDSA recovery silently ignores.
Choosing MAX_SIGNATURE_SIZE = 68 bounds the previously unbounded encoding window while remaining compatible with the historical data on chain.
Long-term goal is to converge to a strict == 65 check once the historical compatibility window is no longer needed. The bound is centralized in ChainConstant and the check in SignUtils.isValidLength, so tightening becomes a one-line change.

Predicate placement

The size check lives in SignUtils.isValidLength(int size, boolean osakaAllowed) (crypto module), not in ChainConstant:

The check is a signature concern, not a chain constant.
The crypto module is reachable from every callsite (chainbase, consensus, framework).
A single predicate eliminates duplicated bounds checks across six sites and makes the bound trivially tightenable.

A companion helper DynamicPropertiesStore.isAllowTvmOsaka() replaces the previous inline getAllowTvmOsaka() == 1L so callers thread a boolean, not a long.

PBFT quorum accounting

validPbftSign now dedupes accepted votes by the recovered SR address rather than by raw signature bytes. This keeps quorum accounting robust regardless of the exact byte-level encoding of each submitted signature. A regression test (PbftDataSyncHandlerTest.testValidPbftSignDedupesByAddress) covers the path.

Breaking changes

chainbase module public-method signatures changed to thread DynamicPropertiesStore through the size check:

Method	Before	After
`TransactionCapsule.checkWeight`	`(Permission, List<ByteString>, byte[], List<ByteString>)`	`(..., DynamicPropertiesStore)` (extra trailing arg)
`TransactionCapsule.addSign`	`(byte[], AccountStore)`	`(byte[], AccountStore, DynamicPropertiesStore)`

checkWeight additionally calls Objects.requireNonNull(dynamicPropertiesStore). All in-tree callers are updated. External JVM consumers of the chainbase artifact (SDKs, signing services, third-party tools) must add the new argument; existing call sites will fail to compile rather than silently misbehave.

Changed files

File	Change
`common/.../config/Parameter.java`	Add `MIN_SIGNATURE_SIZE` / `MAX_SIGNATURE_SIZE` to `ChainConstant`
`crypto/.../SignUtils.java`	Add `isValidLength(int, boolean)` predicate
`chainbase/.../DynamicPropertiesStore.java`	Add `isAllowTvmOsaka()` helper
`chainbase/.../TransactionCapsule.java`	Thread `DynamicPropertiesStore` through `checkWeight` / `addSign`; enforce size bounds post-Osaka
`chainbase/.../BlockCapsule.java`	Apply size bounds to witness signatures in `validateSignature`
`framework/.../Wallet.java`	Apply size bounds in `getTransactionApprovedList`
`framework/.../RelayService.java`	Apply size bounds in `checkHelloMessage`
`framework/.../PbftDataSyncHandler.java`	Apply size bounds in `ValidPbftSignTask`; dedupe PBFT quorum by recovered SR address
`framework/.../PbftMsgHandler.java`	Pass `DynamicPropertiesStore` into `analyzeSignature`
`consensus/.../PbftBaseMessage.java`	Apply size bounds in `analyzeSignature`
`consensus/.../PbftMessageHandle.java`	Pass `DynamicPropertiesStore` into `analyzeSignature`
`actuator/.../TransactionUtil.java`	Update `checkWeight` call site

Test plan

TransactionCapsuleTest — checkWeight size enforcement pre/post-Osaka
BlockCapsuleTest — witness signature size enforcement pre/post-Osaka
TransactionExpireTest — getTransactionApprovedList size enforcement pre/post-Osaka
TransactionUtilTest — getTransactionSignWeight size enforcement post-Osaka
PbftDataSyncHandlerTest — PBFT sign size enforcement post-Osaka; quorum dedup (testValidPbftSignDedupesByAddress)
PbftMsgHandlerTest — PBFT message sign size enforcement post-Osaka
RelayServiceTest — hello message sign size enforcement post-Osaka

coderabbitai · 2026-05-12T12:25:18Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97a7f5b6-a1ec-4a3b-a0ec-00a372971961

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR implements three major feature areas: stricter signature validation with Osaka TVM support, configurable rate limiter blocking modes with per-IP coordination, event configuration initialization reordering, plugin version gating, and defensive null checks. The changes span 42 files across core transaction/block handling, rate limiting, configuration, and test infrastructure.

Changes

Signature Size Validation with Osaka TVM Support

Layer / File(s)	Summary
TransactionCapsule signature validation contract `chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java`	`checkWeight(...)` signature is updated to accept `DynamicPropertiesStore` parameter; the method enforces exact 65-byte signatures when `getAllowTvmOsaka() == 1` alongside existing minimum-size checks.
BlockCapsule and Wallet validation `chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java`, `framework/src/main/java/org/tron/core/Wallet.java`	Block witness and transaction approved-list signatures are validated with the same Osaka-gated exact-size rule, rejecting undersized or non-compliant signatures before address derivation.
Call site updates and test suite `actuator/src/main/java/org/tron/core/utils/TransactionUtil.java`, `framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java`, `framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java`, `framework/src/test/java/org/tron/core/db/TransactionExpireTest.java`	TransactionUtil passes null for the new parameter; tests verify rejection/acceptance of short, padded, and valid 65-byte signatures across pre/post-Osaka configurations.

Rate Limiter Blocking/Non-Blocking Mode

Layer / File(s)	Summary
Rate limiter interface and strategy contracts `framework/src/main/java/org/tron/core/services/ratelimiter/adapter/IRateLimiter.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/strategy/QpsStrategy.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/strategy/IPQpsStrategy.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/strategy/GlobalPreemptibleStrategy.java`	`IRateLimiter` adds `acquire()` and default `acquirePermit()` dispatch based on `Args.isRateLimiterApiNonBlocking()`; strategy classes implement blocking acquisition with timeout handling and per-IP limiter centralization via `loadLimiter()` helpers.
Rate limiter adapters `framework/src/main/java/org/tron/core/services/ratelimiter/adapter/DefaultBaseQqsAdapter.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/adapter/GlobalPreemptibleAdapter.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/adapter/IPQPSRateLimiterAdapter.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/adapter/QpsRateLimiterAdapter.java`	All adapters add `acquire()` method overrides that delegate to their underlying strategies, enabling consistent blocking-mode dispatch across the adapter layer.
GlobalRateLimiter refactoring and HTTP/gRPC interceptors `framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java`, `framework/src/main/java/org/tron/core/services/http/RateLimiterServlet.java`, `framework/src/main/java/org/tron/core/services/ratelimiter/RateLimiterInterceptor.java`	`GlobalRateLimiter.loadIpLimiter()` helper centralizes per-IP limiter creation and coordinates IP-before-global acquisition ordering; servlet and interceptor switch from `tryAcquire()` to `acquirePermit()` to enable runtime blocking-mode selection.
Rate limiter configuration `common/src/main/java/org/tron/core/config/args/RateLimiterConfig.java`, `common/src/main/java/org/tron/common/parameter/CommonParameter.java`, `common/src/main/resources/reference.conf`, `framework/src/main/resources/config.conf`	`RateLimiterConfig` and `CommonParameter` add `apiNonBlocking` boolean field; configuration files introduce the flag with documentation describing blocking (legacy) vs non-blocking (immediate rejection) semantics.
Rate limiter test coverage `framework/src/test/java/org/tron/core/services/ratelimiter/GlobalRateLimiterTest.java`, `framework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.java`, `framework/src/test/java/org/tron/core/services/ratelimiter/RateLimiterInterceptorTest.java`, `framework/src/test/java/org/tron/core/services/ratelimiter/adaptor/AdaptorTest.java`, `common/src/test/java/org/tron/core/config/args/RateLimiterConfigTest.java`	Tests verify `acquirePermit()` dispatch logic, per-IP/global limiter interaction and ordering, failure handling when IP limiter cannot be created, blocking vs non-blocking behavior under configuration, and permit release semantics.

Event Configuration Refactoring and Plugin Versioning

Layer / File(s)	Summary
Event configuration application order `framework/src/main/java/org/tron/core/config/args/Args.java`, `common/src/main/java/org/tron/core/config/args/EventConfig.java`	Event config application is moved after CLI overrides; event subscription enablement is computed via logical OR to preserve CLI-set values; contract parsing is configured directly in config loading rather than via `applyEventConfig()`.
Plugin version compatibility `framework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.java`, `framework/src/test/java/org/tron/common/logsfilter/EventLoaderTest.java`	`EventPluginLoader` adds `MIN_PLUGIN_VERSION` constant, `VersionManager` initialization, and `isPluginVersionSupported()` gate; plugins below version 3.0.0 are rejected at startup with error logging.
Event and rate limiter configuration tests `framework/src/test/java/org/tron/core/config/args/ArgsTest.java`, `common/src/test/java/org/tron/core/config/args/RateLimiterConfigTest.java`	Tests verify event plugin config is built when CLI `--es` is supplied, event config is applied after CLI overrides, and rate limiter config parses the new `apiNonBlocking` flag.

Defensive Improvements and Test Infrastructure

Layer / File(s)	Summary
Null safety and test lifecycle `framework/src/main/java/org/tron/core/net/peer/PeerConnection.java`, `framework/src/test/java/org/tron/core/net/messagehandler/ChainInventoryMsgHandlerTest.java`	`PeerConnection.setChannel()` guards relay node detection against null; `ChainInventoryMsgHandlerTest` adds `@BeforeClass`/`@AfterClass` lifecycle for test configuration isolation.

🎯 4 (Complex) | ⏱️ ~75 minutes

A rabbit hops through signature fences,
Rate limits now block or bend to sense,
Events reorder their dance with care,
Plugin versions checked with flair,
Each byte and limiter finds its place! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.81% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(chainbase): limit oversized signatures' directly summarizes the main change—enforcing signature size validation (specifically limiting signatures to exactly 65 bytes) when TVM Osaka is enabled, which is the core security fix in this PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/sig_length_limit

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cubic-dev-ai

1 issue found across 15 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java">

<violation number="1" location="chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java:245">
P1: Do not require signatures to be exactly 65 bytes here; this breaks compatibility for valid signatures that include trailing bytes.

(Based on your team's feedback about preserving transaction signature compatibility by accepting signatures of at least 65 bytes.) [FEEDBACK_USED]</violation>
</file>

_{Tip: Review your code locally with the cubic CLI to iterate faster.}

cubic-dev-ai

1 issue found across 13 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.java">

<violation number="1" location="framework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.java:179">
P1: Allowing 66–68 byte PBFT signatures here enables trailing-byte variants of the same signature to be counted as distinct votes, because quorum is deduplicated by raw signature bytes while recovery uses only the first 65 bytes.</violation>
</file>

_{Tip: Review your code locally with the cubic CLI to iterate faster.}

Allowing 65-68 byte PBFT signatures permitted trailing-byte variants of the same physical signature to be counted as distinct votes: the quorum set was keyed by raw signature bytes while ECDSA recovery only consumes the first 65 bytes, so a single signer could submit padded copies and inflate their vote weight. Switch validPbftSign to count unique recovered SR addresses instead, and add a regression test exercising sigs padded to 65/66/67/68 bytes from the same signer. Also extract the signature length predicate into SignUtils.isValidLength to consolidate the six callsites that gate on the TVM Osaka bound.

Federico2014 changed the title ~~fix(chainbase): reject oversized signatures after TVM Osaka activation~~ fix(chainbase): limit oversized signatures May 12, 2026

Federico2014 changed the base branch from develop to release_v4.8.2 May 12, 2026 12:28

Federico2014 changed the title ~~fix(chainbase): limit oversized signatures~~ fix(chainbase): strengthen signature length validation May 12, 2026

Federico2014 force-pushed the fix/sig_length_limit branch from bf862a9 to e1df760 Compare May 12, 2026 12:31

fix(chainbase): strengthen signature length validation

bc74d53

Federico2014 force-pushed the fix/sig_length_limit branch from 922269a to bc74d53 Compare May 12, 2026 12:39

cubic-dev-ai Bot reviewed May 13, 2026

View reviewed changes

Comment thread chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java Outdated

Federico2014 force-pushed the fix/sig_length_limit branch from 390e342 to 9a6c149 Compare May 13, 2026 08:57

fix(chainbase): align signature size checks

6d5fcce

Federico2014 force-pushed the fix/sig_length_limit branch from 9a6c149 to 6d5fcce Compare May 13, 2026 09:03

refactor(common): extract signature size bounds to constants

9150745

cubic-dev-ai Bot reviewed May 15, 2026

View reviewed changes

Comment thread framework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.java Outdated

Sunny6889 reviewed May 15, 2026

View reviewed changes

Comment thread chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java Outdated

Federico2014 force-pushed the fix/sig_length_limit branch from 769d945 to 14cfb24 Compare May 15, 2026 07:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(chainbase): strengthen signature length validation#27

fix(chainbase): strengthen signature length validation#27
Federico2014 wants to merge 4 commits into
release_v4.8.2from
fix/sig_length_limit

Federico2014 commented May 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 12, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Federico2014 commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design notes

Size bound: 65..68 instead of strict 65

Predicate placement

PBFT quorum accounting

Breaking changes

Changed files

Test plan

Uh oh!

coderabbitai Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Federico2014 commented May 12, 2026 •

edited

Loading

coderabbitai Bot commented May 12, 2026 •

edited

Loading